Skip to content

fix(verify/sev_snp,tpm): enforce canonical lowercase hex in measurement checks and validate report_data_hex (#811) - #816

Open
drk-ram wants to merge 1 commit into
agentrust-io:mainfrom
drk-ram:drk-ram-fix-snp-tpm-verify-811
Open

drk-ram wants to merge 1 commit into
agentrust-io:mainfrom
drk-ram:drk-ram-fix-snp-tpm-verify-811

Conversation

@drk-ram

@drk-ram drk-ram commented Oct 25, 2026

Copy link
Copy Markdown

What

Enforce canonical lowercase hex in verify_sev_snp_measurement and tpm._valid_measurement, and validate report_data_hex format and length (<= 128 hex chars) before decoding in verify_sev_snp_measurement.

Why

Closes #811.

Security impact

Prevents non-hex "sha384:" + "z" * 96 strings from passing Step 1 format checks in verify_sev_snp_measurement, prevents silent truncation of oversized report_data_hex[:128], and prevents malformed report_data_hex from being misreported as raw_evidence_parse_error.

Test plan

  • pytest passes (pytest tests/unit/test_sev_snp_verify.py -q: 22 passed)
  • ruff check passes (All checks passed!)
  • mypy passes
  • Manual test performed

DCO sign-off

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Oct 25, 2026

@noah-ing noah-ing left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 87498ac against #811. Independent synthetic SNP/TPM controls reproduce the previous acceptance of an overlong report_data_hex and the previous misclassification of noncanonical measurement hex; this head rejects them while preserving a valid lowercase measurement and 128-character report-data control. The focused checks passed (50 passed, 1 skipped), as did the broader unit, conformance, integration, and confinement suite (2,640 passed, 43 skipped), lint, typing, and dependency checks.

I approve this fix within that local verification scope. The synthetic SNP control did not include a VCEK certificate chain, so this review does not claim hardware attestation. Normal hosted checks and maintainer approval remain applicable.

@carloshvp carloshvp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 87498ac6853e59e43ebd7cc5d5b9a3f078dbd2e9 in an isolated checkout.

The SNP/TPM measurement guards require the declared lowercase digest formats, and report-data input is validated before decoding rather than silently truncating oversized values. The focused format/report-data regression passes here and fails with the actual baseline verifier modules restored in process. Valid report/measurement controls remain passing; this review does not establish new hardware execution.

Independent validation in a fresh repository-hash-locked Python 3.12.10 environment: 2,526 tests passed, 20 skipped; pip check, configured Ruff, mypy, Bandit and diff whitespace checks passed. Commit sign-offs are present. Hosted CI and applicable independent maintainer/security gates remain separate prerequisites to merge.

Reviewed with OpenAI Codex.

…nt checks and validate report_data_hex (agentrust-io#811)

Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com>
@drk-ram
drk-ram force-pushed the drk-ram-fix-snp-tpm-verify-811 branch from 87498ac to f06f3ff Compare October 31, 2026 15:53

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

3 participants